pull: Don't save summary to cache before validating signatures

In case of some kind of race or other weirdness we might be getting
non-matching versions of summary.sig and summary, where summary.sig
is the latest version. Currently we're saving them to the cache
directly after downloading them successfully, but they will then fail
to gpg validate. Then on the next run we'll keep using the cached files
even if they are incorrect, until summary.sig changes upstream.

This changes the order so that we verify the signatures before saving
to the cache, thus ensuring that we don't end up in a stuck state.

Fixes https://github.com/ostreedev/ostree/issues/1523

Closes: #1529
Approved by: cgwalters

diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c
index 37659459a389ba277f28790064fd1ce6eb3f7cde..f46616383d0364f8f5e538f6f5eb6330838447bf 100644 (file)
--- a/src/libostree/ostree-repo-pull.c
+++ b/src/libostree/ostree-repo-pull.c
@@ -2913,6 +2913,7 @@ repo_remote_fetch_summary (OstreeRepo    *self,
                            GVariant      *options,
                            GBytes       **out_summary,
                            GBytes       **out_signatures,
+                           gboolean      *out_from_cache,
                            GCancellable  *cancellable,
                            GError       **error)
 {
@@ -3015,32 +3016,13 @@ repo_remote_fetch_summary (OstreeRepo    *self,
         goto out;
     }
 
-  if (!from_cache && *out_summary && *out_signatures)
-    {
-      g_autoptr(GError) temp_error = NULL;
-
-      if (!_ostree_repo_cache_summary (self,
-                                       name,
-                                       *out_summary,
-                                       *out_signatures,
-                                       cancellable,
-                                       &temp_error))
-        {
-          if (g_error_matches (temp_error, G_IO_ERROR, G_IO_ERROR_PERMISSION_DENIED))
-            g_debug ("No permissions to save summary cache");
-          else
-            {
-              g_propagate_error (error, g_steal_pointer (&temp_error));
-              goto out;
-            }
-        }
-    }
-
   ret = TRUE;
 
  out:
   if (mainctx)
     g_main_context_pop_thread_default (mainctx);
+
+  *out_from_cache = from_cache;
   return ret;
 }
 
@@ -5763,6 +5745,7 @@ ostree_repo_remote_fetch_summary_with_options (OstreeRepo    *self,
   g_autoptr(GBytes) signatures = NULL;
   gboolean ret = FALSE;
   gboolean gpg_verify_summary;
+  gboolean summary_is_from_cache;
 
   g_return_val_if_fail (OSTREE_REPO (self), FALSE);
   g_return_val_if_fail (name != NULL, FALSE);
@@ -5777,6 +5760,7 @@ ostree_repo_remote_fetch_summary_with_options (OstreeRepo    *self,
                                   options,
                                   &summary,
                                   &signatures,
+                                  &summary_is_from_cache,
                                   cancellable,
                                   error))
     goto out;
@@ -5813,6 +5797,27 @@ ostree_repo_remote_fetch_summary_with_options (OstreeRepo    *self,
         goto out;
     }
 
+  if (!summary_is_from_cache && summary && signatures)
+    {
+      g_autoptr(GError) temp_error = NULL;
+
+      if (!_ostree_repo_cache_summary (self,
+                                       name,
+                                       summary,
+                                       signatures,
+                                       cancellable,
+                                       &temp_error))
+        {
+          if (g_error_matches (temp_error, G_IO_ERROR, G_IO_ERROR_PERMISSION_DENIED))
+            g_debug ("No permissions to save summary cache");
+          else
+            {
+              g_propagate_error (error, g_steal_pointer (&temp_error));
+              goto out;
+            }
+        }
+    }
+
   if (out_summary != NULL)
     *out_summary = g_steal_pointer (&summary);